Support setting a field type with SET DATA TYPE statement in engine and Iceberg
#18395
Conversation
github-actions
bot
added
tests:hive
iceberg
ebyhr
force-pushed
the
ebi/core-field-set-data-type
branch
from
783327d
to
9e4b521
Compare
ebyhr
force-pushed
the
ebi/core-field-set-data-type
branch
2 times, most recently
from
4982e49
to
31e4be9
Compare
| String caseSensitiveParentName = icebergTable.schema().findColumnName(parent.fieldId()); | ||
| NestedField field = parent.type().asStructType().caseInsensitiveField(getLast(fieldPath)); | ||
| if (!field.type().isPrimitiveType()) { | ||
| throw new TrinoException(NOT_SUPPORTED, "Iceberg doesn't support changing field type from non-primitive types"); |
There was a problem hiding this comment.
Do we want to disallow type changes between primitive and nested at engine level?
There was a problem hiding this comment.
Even if we do this, connector will have to check that as well. Engine cannot do such checks atomically.
| throw new TrinoException(NOT_SUPPORTED, "Unsupported type: " + type); | ||
| } | ||
| List<RowType.Field> candidates = rowType.getFields().stream() | ||
| // case-insensitive match |
There was a problem hiding this comment.
Can this be case sensitive? (eg not to get us one step further from #17)
There was a problem hiding this comment.
This is the same logic as row fields resolution in expressions as you may know.
@kasiafi requested a case-insensitive match in adding fields PR #16321 (comment).
Currently, adding & renaming fields are case-insensitive and dropping field (this was the 1st PR about managing nested fields) is case-sensitive. We should change dropping fields to case-insensitive in the near future.
core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMetadata.java
Show resolved
Hide resolved
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java
Show resolved
Hide resolved
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java
Show resolved
Hide resolved
| String caseSensitiveParentName = icebergTable.schema().findColumnName(parent.fieldId()); | ||
| NestedField field = parent.type().asStructType().caseInsensitiveField(getLast(fieldPath)); | ||
| if (!field.type().isPrimitiveType()) { | ||
| throw new TrinoException(NOT_SUPPORTED, "Iceberg doesn't support changing field type from non-primitive types"); |
There was a problem hiding this comment.
Even if we do this, connector will have to check that as well. Engine cannot do such checks atomically.
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java
Show resolved
Hide resolved
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java
Show resolved
Hide resolved
ebyhr
force-pushed
the
ebi/core-field-set-data-type
branch
from
31e4be9
to
4c6319e
Compare
Description
This is the final PR about managing nested fields in the engine.
Fixes #16959
Release notes
(x) Release notes are required, with the following suggested text: